Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gpio Interrupt fix for the CC2538 #7303

Merged
merged 1 commit into from
Jul 4, 2017
Merged

Conversation

AnonMall
Copy link

@AnonMall AnonMall commented Jul 3, 2017

Fixed the issue where the gpio interrupt is not properly acknowledged.

@smlng smlng added Area: drivers Area: Device drivers Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Jul 3, 2017
@smlng
Copy link
Member

smlng commented Jul 3, 2017

please squash (and rebase if necessary?), otherwise ACK!

@aabadie
Copy link
Contributor

aabadie commented Jul 3, 2017

Does this PR provides a fix for #6650 ? Then it should be merged asap.

@aabadie aabadie added this to the Release 2017.07 milestone Jul 3, 2017
@smlng
Copy link
Member

smlng commented Jul 3, 2017

partly - the issue is more complex, #6773 provides the other part, but is more a workaround than a fix. The problem is that the gpio periph of cc2538 was partly adapted to periph_gpio driver of RIOT, but i2c, spi, and uart are still using the gpio driver from TI. Hence, rather than applying #6773 workaround, I'd work on rewritting the named 3 periph drivers ...

Nevertheless, this PR fixes an issue when using interrupts on gpios

@smlng
Copy link
Member

smlng commented Jul 3, 2017

But still: DO NOT MERGE THIS, just to have in the release. We had lots of trouble lately with prematurely merged periph drivers - this need very careful testing!

Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs testing, DO NOT MERGE!

@smlng smlng self-assigned this Jul 3, 2017
@smlng smlng modified the milestones: Release 2017.10, Release 2017.07 Jul 3, 2017
@aabadie
Copy link
Contributor

aabadie commented Jul 3, 2017

DO NOT MERGE!

no problem, it won't be in the release if it's not ACK'ed, merged and backported. Keep calm ;)

@AnonMall
Copy link
Author

AnonMall commented Jul 3, 2017

sorry i am a little new to squashing... just fixing some things. Please bear with me.

@AnonMall AnonMall force-pushed the gpioIntFix branch 3 times, most recently from 2dca87c to ddc7c2b Compare July 3, 2017 20:37
@smlng
Copy link
Member

smlng commented Jul 3, 2017

take your time, consider the following:

  1. setup RIOT-OS/RIOT as an upstream to your repo, see here
  2. update your master to upstream master:
git fetch upstream
git checkout master
git pull --rebase upstream master
  1. now update your branch and push (force, is needed as you rewrite history of your branch):
git checkout  gpioIntFix
git rebase master
git push -f
  1. in case you have commit you want to squash use interactive rebase, see here too
git rebase -i master

@miri64
Copy link
Member

miri64 commented Jul 3, 2017

In fact you can just git fetch upstream and rebase to upstream/master (git rebase upstream/master or git rebase -i upstream/master for interactive mode). edit: shouldn't spewing improvements full of errors themselves ^^"

@AnonMall AnonMall force-pushed the gpioIntFix branch 2 times, most recently from 4bc42d5 to c4584ac Compare July 3, 2017 22:00
@AnonMall
Copy link
Author

AnonMall commented Jul 4, 2017

fnished rebase and should be squashed... as it is only one file, further rebasing shouldn't be nescessary i guess. Thank you both for helping out.

@smlng
Copy link
Member

smlng commented Jul 4, 2017

perfect! I'll test.

@smlng smlng added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Jul 4, 2017
@smlng
Copy link
Member

smlng commented Jul 4, 2017

tested this PR on board remote-revb and it fixes the following issue present on master:

  • GPIO init as interrupt works as expected
  • but when interrupt fires, it does not return correctly
  • for tests/periph_gpio this means no shell prompt after interrupt

With this PR interrupt handling returns and shell prompt is available again.

Nice job @AnonMall, thanks! ACK - and GO as soon as Murdock is happy.

@smlng smlng added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 4, 2017
@miri64
Copy link
Member

miri64 commented Jul 4, 2017

Maybe, if it fixes some stuff we should reconsider backporting into the release. What do you think @aabadie?

@aabadie
Copy link
Contributor

aabadie commented Jul 4, 2017

I think it should be backported (since the mentioned bugs are quite important)

@smlng smlng merged commit e91c077 into RIOT-OS:master Jul 4, 2017
@smlng smlng modified the milestones: Release 2017.07, Release 2017.10 Jul 4, 2017
@miri64 miri64 added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jul 4, 2017
@smlng
Copy link
Member

smlng commented Jul 4, 2017

@AnonMall can you provide the backport? Basically make a copy of the branch and rebase onto tag 2017.07-RC1, then make a new pull request against 2017.07-branch (instead of master).

[edit]: and prefix the PR title with [backport] to make that clear.

@AnonMall
Copy link
Author

AnonMall commented Jul 4, 2017

I can do that. But that also means you want me to rebase it against 2017.07-branch instead of master right?

@AnonMall
Copy link
Author

AnonMall commented Jul 4, 2017

Okay done. #7314

@miri64 miri64 removed the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Oct 18, 2017
@AnonMall AnonMall deleted the gpioIntFix branch January 31, 2018 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants